[rdma-core] Add new port#51531
Conversation
BillyONeal
left a comment
BillyONeal
left a comment
There was a problem hiding this comment.
I asked GPT 5.4 to look at this. About half the report it generated was nonsense but these bits seem reasonable. In particular the license bits probably need to be fixed.
"ccan" being vendored may be fine if this is one of those situations where we would never expect that to be provided by itself.
The hard "request changes" is about the licensing parts.
===========================
Build Invocation Summary
- Key options:
-DENABLE_RESOLVE_NEIGH=OFF,-DNO_MAN_PAGES=ON,-DNO_PYVERBS=ON,-DENABLE_STATIC=...(ports/rdma-core/portfile.cmake:20-38) - Feature controls: tests, examples, docs, infiniband-diags,
srp_daemon,kernel-boot, andlibrspreloadare patched out (ports/rdma-core/0001-disable-tests.patch:1-28,0002-disable-examples.patch:1-13,0003-disable-documentation.patch,0004-disable-infiniband-diags.patch:1-15,0005-disable-srp-daemon.patch:1-12,0006-disable-kernel-boot.patch:1-13,0007-disable-librspreload.patch:1-33) - Platform guards:
supports: "linux"in metadata; install succeeded on Linux - Observed configure command: static-only build on
x64-linux, withVCPKG_LOCK_FIND_PACKAGE_Systemd=OFFandVCPKG_LOCK_FIND_PACKAGE_UDev=OFF(buildtrees/rdma-core/config-x64-linux-out.log:1)
Vendored Dependencies
-
Bundled component:
ccan- Evidence: upstream builds an internal static
ccanlibrary and links it throughCOMMON_LIBS/COMMON_LIBS_PIC(buildtrees/rdma-core/src/v62.0-f5edcc20ee.clean/ccan/CMakeLists.txt:1-22,buildlib/rdma_functions.cmake:13-15,103-115,247-301) - Status: present and folded into installed static libraries, but not installed as standalone headers or a separate library
- Assessment: vendored internal code is materially part of shipped
.aarchives, and it carriesMIT/CC0notices (buildtrees/rdma-core/src/v62.0-f5edcc20ee.clean/COPYING.md:26-35,ccan/LICENSE.MIT,ccan/LICENSE.CCO)
- Evidence: upstream builds an internal static
-
Bundled provider-specific code
- Evidence: installed static provider archives include
libipathverbs-rdmav59.a,libhfi1verbs-rdmav59.a,libocrdma-rdmav59.a(packages/rdma-core_x64-linux/lib/...) - Status: installed
- Assessment: upstream documents provider-specific alternate licenses beyond the default dual license (
COPYING.md:36-49,providers/ipathverbs/COPYING:1-35,COPYING.BSD_FB:1-22)
- Evidence: installed static provider archives include
License / Installed Content Findings
- Finding: installed content appears broader than the declared SPDX expression
- Declared license:
GPL-2.0-only OR Linux-OpenIB - Observed installed content: static provider archives and internally linked vendored
ccancode inpackages/rdma-core_x64-linux/lib/*.a - Evidence: upstream license catalog calls out
ccanunderMIT/CC0and provider-specific alternatives including Intel 3-clause BSD, PathScale BSD patent, and OpenIB BSD FreeBSD variant (buildtrees/rdma-core/src/v62.0-f5edcc20ee.clean/COPYING.md:26-49;providers/ipathverbs/COPYING:1-35;COPYING.BSD_FB:1-22) - Assessment:
vcpkg.jsonlikely under-describes the installed licensing surface. At minimum, the copyright bundle is incomplete for the installed static-provider payload, and the SPDX expression may need to account for additional bundled-license alternatives
- Declared license:
Other Port Review Suggestions
- Suggestion: review whether static-only packaging should ship all provider archives unconditionally
- Evidence: the package installs many provider-specific static libraries with no features (
packages/rdma-core_x64-linux/lib/*;ports/rdma-core/vcpkg.json:1-23) - Rationale: this is a large payload with diverse licensing and hardware-specific code, but consumers cannot opt out or model providers separately
- Evidence: the package installs many provider-specific static libraries with no features (
|
Despite the nitpicks, thank you for the new port submission! |
Looking at ccan's documentation on its website, it appears that it's designed to be vendored, rather than consumed as a dependency. As such, I think this is best left as-is. Will rework the licensing info. |
BillyONeal
added
the
requires:vcpkg-team-review
|
@BillyONeal - there are a couple of licences (PathScale BSD Patent license and Intel 3 clause BSD license), that are not listed in standalone files within the source repo, and are only listed within headers. How should I handle this? Should I vendor the licenses as files within the port directory, and install them from there? |
|
Even if designed to be vendored that's still broken if 2 ports do this. (We have similar problems with imgui which also expects to be vendored that also breaks the universe as soon as 2 people do this) It looks like for now that isn't creating conflicts so I don't think it's an over our dead bodies issue. But still + requires:vcpkg-team-review for a 2nd opinion |
|
@vicroms "I don't oppose making an exception for this one" (both of us) If it ever becomes a problem we will probably have to write our own custom build system / targets for ccan and devendor from all the places doing that but for now vendoring as upstream is already doing seems the least bad among the available bad options. |
I think you can just add the header to the |
BillyONeal
removed
the
requires:vcpkg-team-review
chris1786
force-pushed
the
rdma-core
branch
2 times, most recently
from
f968715 to
d5ba23e
Compare
|
Notwithstanding that I'm still working on collating the licensing information accurately, I don't understand why the build is currently failing |
2 participants
Owner-Projectform.vcpkg.json, or explicitly disabled through patches or build system arguments such as CMAKE_DISABLE_FIND_PACKAGE_Xxx or VCPKG_LOCK_FIND_PACKAGEvcpkg.jsonmatches what upstream says.vcpkg.jsonmatches what upstream says../vcpkg x-add-version --alland committing the result.